Skip to content

Debounce of the digests triggered by store modification #175

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 15 commits into from
Apr 24, 2018

Conversation

jtassin
Copy link
Contributor

@jtassin jtassin commented Nov 16, 2017

capability for ngRedux to debounce the digests

The problem

In my huge app, I have a lot of stimulations that modify the store and each one lead to a digest (via $rootScope.$evalAsync). The accumulation of digests considerably affect the performance of the application.

Solution

We added a use of lodash.debounce in the digest middleware to avoid digesting too much.

Content of the PR

  • add lodash.debounce to the dependencies
  • Use of lodash.debounce in the digest middleware
  • Creation of a config for ngRedux
  • Documentation of the config
  • unit test for the debounce
  • Correction of a problem with redux-thunk in ngRedux.js

Use of the fonctionnality in my app

import angular from 'angular';

angular.module('ngapplication').config(($ngReduxProvider) => {
  'ngInject';

  // eslint-disable-next-line
  $ngReduxProvider.config.debounce = {
    wait: 250,
    maxWait: 500,
  };
});

last but not least

Thx for your usefull lib ;-)

@@ -41,6 +41,13 @@ export default function ngReduxProvider() {
_initialState = initialState || {};
};

this.config = {
debounce: {
wait: undefined,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if we do a default debounce of 1ms, would it be possible for us to batch updates "per tick" essentially? I'm thinking of how you might queue several actions at the same time and instead of digesting each one immediately, we'd let it all run on the next tick/frame/whatever.

I'm not sure if $evalAsync does this already or not.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The "per tick" should be already covered by the $evalAsync.

I think that 1ms won't make a real performance gain.

Here we set it to {wait: 100, maxWait: 500} for a gain of ~50% of digests.

@AntJanus
Copy link
Collaborator

@jtassin this is fantastic functionality! I'd like to add this ASAP. Can you look over it once more? I definitely let this one linger for way too long.

README.md Outdated
// eslint-disable-next-line
$ngReduxProvider.config.debounce = {
wait: 100,
mawWait: 500,

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like there's a typo here, I assume this is intended to be maxWait

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right, thx for review.
It's corrected.

@AntJanus AntJanus merged commit 87d1808 into angular-redux:master Apr 24, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants